Support --learn for inline expectation tests - #22228
Open
d10c wants to merge 1 commit into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * The `--learn` postprocessing needs to see *every* parsed expectation on a comment - including | ||
| * ones the running test ignores - so it can preserve them when rewriting. | ||
| */ | ||
| predicate getAnExpectation( |
Contributor
Author
There was a problem hiding this comment.
This is just a move of an existing predicate; thought it would be better to leave it as-is. But I can rename it if needed.
d10c
force-pushed
the
d10c/learn-inline-expectations-lib
branch
2 times, most recently
from
July 28, 2026 15:10
92e2729 to
63edc17
Compare
`codeql test run --learn` could previously only rewrite `.expected`
files; inline expectations (the `// $ Alert` comments checked by
`InlineExpectationsTest`) had to be fixed by hand. This teaches the
shared test library to compute those source edits so the test runner
can apply them.
The `test-postprocess` query now exposes a `learnEdits` relation
(`file, line, operation, startColumn, endColumn, text`) describing the
minimal source rewrite that would make the inline expectations match the
actual query results. It covers:
- appending a fresh comment carrying every tag learned for a line that
has an unexpected result and no existing comment to merge into;
- rewriting an existing expectation comment as a whole so it matches
the current results: dropping fixed-spurious tags, promoting a
`MISSING:` expectation that now fires, clearing a stale `SPURIOUS:`
annotation, and merging in freshly learned tags, re-rendering the
remaining expectations (or deleting the comment when none remain);
- preserving expectations this test does not own -- e.g. a tag
annotated with a different query's id that shares the source file --
and any trailing regular note (`// $ Alert // note`);
- recording any unexpected result, not just `Alert`.
Edits are emitted as a query predicate rather than applied here: the
engine consumes `learnEdits` only under `--learn` and ignores it
otherwise, so ordinary `test run` output is unchanged.
Comment syntax is provided per language by each
`InlineExpectationsTestQuery.ql`'s `Input` (`getStartCommentMarker`),
which for now renders only line-comment languages; block-comment
languages (XML/YAML) can supply their markers later without changing
this relation's shape.
d10c
force-pushed
the
d10c/learn-inline-expectations-lib
branch
from
July 28, 2026 16:03
63edc17 to
52f2ac6
Compare
d10c
marked this pull request as ready for review
July 28, 2026 16:17
Contributor
There was a problem hiding this comment.
Pull request overview
Adds inline --learn support so test expectations can be rewritten directly in source files.
Changes:
- Adds structured
learnEditsgeneration and expectation-preserving rewrite logic. - Introduces per-file comment-marker APIs.
- Configures supported source extensions for each language.
Show a summary per file
| File | Description |
|---|---|
shared/util/codeql/util/test/InlineExpectationsTest.qll |
Implements parsing and learned edits. |
cpp/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables C/C++ line comments. |
csharp/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables C# line comments. |
go/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables Go line comments. |
java/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables Java/Kotlin line comments. |
javascript/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables JavaScript/TypeScript line comments. |
python/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables Python line comments. |
ql/ql/src/utils/test/InlineExpectationsTestQuery.ql |
Enables QL/dbscheme line comments. |
ruby/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables Ruby line comments. |
rust/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables Rust line comments. |
swift/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables Swift line comments. |
unified/ql/lib/utils/test/InlineExpectationsTestQuery.ql |
Enables unified Swift line comments. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Medium
Comment on lines
+675
to
+676
| * inline expectations use line comments; block-comment languages can override it so that | ||
| * `--learn` renders a closing marker. |
aschackmull
reviewed
Jul 29, 2026
| * `--learn` renders a closing marker. | ||
| */ | ||
| bindingset[relativePath] | ||
| default string getEndCommentMarker(string relativePath) { result = "" } |
Contributor
There was a problem hiding this comment.
It would be more natural to use none() here as the default result for comments without an end marker. Otherwise, this is likely to break as soon as, say, html comment markers are added to a language that already has // comments for other files, since it's natural to leave out the result = "" disjunct in those other cases.
aschackmull
reviewed
Jul 29, 2026
Comment on lines
+1079
to
+1084
| endMarker = Input2::getEndCommentMarker(relativePath) and | ||
| ( | ||
| endMarker = "" and endSuffix = "" | ||
| or | ||
| endMarker != "" and endSuffix = " " + endMarker | ||
| ) and |
Contributor
There was a problem hiding this comment.
Suggested change
| endMarker = Input2::getEndCommentMarker(relativePath) and | |
| ( | |
| endMarker = "" and endSuffix = "" | |
| or | |
| endMarker != "" and endSuffix = " " + endMarker | |
| ) and | |
| ( | |
| endSuffix = " " + Input2::getEndCommentMarker(relativePath) | |
| or | |
| endSuffix = "" and not exists(Input2::getEndCommentMarker(relativePath)) | |
| ) and |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Teaches the shared inline-expectations test library to drive
codeql test run --learnfor inline// $ Alertexpectations: when a postprocess test query is run under--learn, the library now emits alearnEditsrelation describing how to rewrite the annotated source so the comments match the actual query results.Requires the matching CLI support for consuming
learnEdits; the library change is incompatible CLIs older than that. The CI on this PR should pass only once a supported CLI gets released to nightly.What it does
For each result/expectation mismatch on the standard
// $ tag1 tag2=value tag3[query] // rest of comment…-style tags,learnEditsrecords a structured edit (append a new comment, remove or rewrite an existing one). The CLI applies these edits to the source file, so--learnupdates the inline expectations directly rather than writing a failure grid into.expected. Without--learnthe relation is invisible and behaviour is unchanged.API
TestPostProcessing::InputSiggainsgetStartCommentMarker(...)(and a defaultedgetEndCommentMarker), keyed on the source file of a location so a single database can mix languages with different comment syntax. Each per-languageutils/test/InlineExpectationsTestQuery.qlsupplies its marker (//,#, ...); block-comment languages are left for a follow-up but the end-marker hook is already in place.Follow-ups not implemented in this PR
// $ Alert[query1] Alert[query2])tag1,tag2=valueshorthand where only one tag needs to be updated.